Fix stale-token 403s: check JWT expiry and refresh proactively - #13
Merged
Conversation
…ry call getToken() validated the iss claim but never checked exp, so an expired token was returned as valid forever. That broke userIsLoggedIn()'s only trigger condition (getToken() == null), making the PATCH /session refresh a no-op once a session existed — exactly the failure NavGraph's per-route call was meant to prevent. waitForToken(), the single choke point both AuthInterceptor (REST) and RealtimeClient (WebSocket) go through to attach auth, now actively calls userIsLoggedIn() whenever it has no valid cached token, instead of passing waiting on a session-established flow that nothing was flipping. This covers every HTTP/WS call without needing to sprinkle userIsLoggedIn() calls across call sites, since apiService and RealtimeClient are the only two network egress paths in the app.
A local-only getToken() != null check can't recover from an expired token the way userIsLoggedIn() can (it triggers PATCH /session), so gating auth-required navigation on the sync check risked landing on LOGIN or APP based on stale state. Both RootNavGraph call sites now call the suspend userIsLoggedIn() instead.
Covers every success and failure mode of AuthressLoginClient (token expiry/refresh, the full authenticate()/completeAuthenticationRequest() PKCE + deep-link flow including abandoned/mismatched/duplicate redirect edge cases, logout, linkIdentity, profile/devices), JwtManager's decode and anti-abuse hash, AuthInterceptor's header attachment, and RealtimeClient's handshake/reconnect/account-switch behavior — the exact surface behind the 403 bug fixed on this branch. Tests run against a real MockWebServer (redirected via a test-only interceptor, since the client's host is a fixed BuildConfig value) with AuthressCookieJar/AuthStorageManager faked via mockk rather than their real EncryptedSharedPreferences-backed implementations. Robolectric provides the android.util.Base64 (JWT decode) and Context (CustomTabsIntent launch) support the client needs; it isn't required for AuthInterceptor or RealtimeClient, which stay plain JUnit. AuthStorageManager is now injected into AuthressLoginClient the same way cookieJar already is, so tests can substitute a fake instead of touching encrypted storage.
…ckk imports
Both are members of MockKAnswerScope, resolved via the answers{} block's
implicit receiver — importing them as free functions doesn't compile.
Switched the pending-auth-request stub to any()+firstArg() (any() already
matches null), and dropped the bogus secondArg import since the bare call
inside answers{} was already correct.
- Switch runTest to runBlocking across the AuthressLoginClient/JwtManager test suites: runTest's virtual-time scheduler auto-advances past the real Dispatchers.IO network call inside AuthressLoginClient.execute(), which made withTimeoutOrNull in waitForToken() fire its timeout before the real MockWebServer response arrived (seen failing: "waitForToken refreshes an expired token and returns the new one"). These tests do real I/O and never use virtual-time control, so runBlocking is the correct tool. - RealtimeClientTest: MockWebServer.shutdown() can throw IOException in tearDown when a WebSocket's close handshake (from client.stop()) hasn't finished yet. Harmless — wrap in runCatching.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Investigated widespread 403s on
/api/accounts/{id}/threads,/resources, and the Realtime WebSocket. Root cause:AuthressLoginClient.getToken()validated theissclaim but never checkedexp, so an already-expired-but-well-formed JWT was returned as valid indefinitely. That silently brokeuserIsLoggedIn()'s only refresh trigger (getToken() == null), soPATCH /sessionstopped firing once a session existed — exactly the failureNavGraph's per-routeuserIsLoggedIn()call was meant to prevent. Both the REST client (AuthInterceptor) and the WebSocket client (RealtimeClient) pull the token through the samewaitForToken()path, so they failed identically and simultaneously.getToken()now rejects a token whoseexp(already shortened byJwtManager's 10s clock-skew buffer) has passed.waitForToken()— the single choke point bothAuthInterceptor(REST) andRealtimeClient(WebSocket) go through to attach auth — now actively callsuserIsLoggedIn()(which triggersPATCH /session) whenever it has no valid cached token, instead of passively waiting on asessionEstablishedflow that nothing was flipping.userIsLoggedIn()no-ops (no network call) whenever a valid token is already cached, so this doesn't add overhead to the common case.userIsLoggedIn()calls across individual call sites:apiService(all Email API calls, viaAuthInterceptor) andRealtimeClientare the only two network egress paths (verified —AppContainer.ktis the only place building anOkHttpClient/Retrofitinstance), and both already route throughwaitForToken().AuthInterceptorandNavGraphthat no longer matched the new behavior.Test plan
:app:assembleDebugand:app:testDebugUnitTestpass in CI.PATCH /sessionrefresh instead of a 403.SyncForegroundService,RealtimeClient's backoff) recover cleanly after an expired token instead of looping on 403s.Generated by Claude Code